Skip to content

feat(workflow): CLI worker, worktrees, resume, schema#97

Draft
Waishnav wants to merge 2 commits into
pr/dw-3-enginefrom
pr/dw-4-cli-worker
Draft

feat(workflow): CLI worker, worktrees, resume, schema#97
Waishnav wants to merge 2 commits into
pr/dw-3-enginefrom
pr/dw-4-cli-worker

Conversation

@Waishnav

Copy link
Copy Markdown
Owner

Summary

  • Named/file script resolve + persist under stateDir
  • Agent worktree factory (wf/<runId>/cN)
  • Resume replay (callIndex+key, consume-once by key)
  • Ajv schema enforcement on agent({ schema })
  • CLI: workflow run|status|cancel|ls|__worker (detached worker, heartbeat/cancel)

Stack

PR4 of 5 · base: pr/dw-3-engine

Test plan

  • tsx src/workflow-files.test.ts
  • tsx src/workflow-replay.test.ts
  • tsx src/workflow-schema.test.ts
  • ./node_modules/.bin/tsx src/cli.ts workflow help
  • Manual: workflow run --file … with fake/available provider; status / cancel
  • npm run typecheck

Waishnav and others added 2 commits July 21, 2026 17:26
Named/file script resolve, agent worktree factory, resume matcher
(index+key then consume-once), and Ajv schema enforcement wired into
agent(). Adds ajv dependency.

Co-Authored-By: Claude <noreply@anthropic.com>
Detached __worker with heartbeat/cancel, real adapters via
runLocalAgentProvider, worktree isolation, and resume wiring.
setScriptPath persists script after run create.

Co-Authored-By: Claude <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: dc5f43da-d6d1-47cd-91bd-517772063332

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch pr/dw-4-cli-worker

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Comment thread src/workflow-cli.ts
});

const persisted =
priorScriptPath ??

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P0] Resume reuses the prior mutable script path and does not verify that the new run is in the same repository/workspace/base SHA. This can replay repository A in B, and prior.scriptHash can disagree with edited bytes executed later. Snapshot the exact validated source under the new run ID and require a matching repository identity/base revision (or an explicit override) before replay.

Comment thread src/workflow-cli.ts
console.log(formatRunLine(run));
if (run.pid && (run.status === "running" || run.status === "starting")) {
try {
process.kill(run.pid, "SIGTERM");

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P0] Sending SIGTERM immediately after setting the cooperative flag usually kills the worker before it can observe cancellation, clean up worktrees, or write terminal events. Please centralize CLI/MCP cancellation in a supervisor: request cancel, wait a grace period, then use the cross-platform terminateProcessTree fallback and atomically mark the run cancelled. The supervisor should also schedule reapStale().

Comment thread src/workflow-cli.ts
}
}

store.completeRun(runId, { resultJson });

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] The run becomes completed before run_completed is appended. A status poll can observe terminal state and stop before the terminal event exists. Please update terminal status and append the corresponding terminal event in one SQLite transaction; use the same path for failed and cancelled runs.

Comment thread src/workflow-schema.ts
? basePrompt
: `${basePrompt}\n\nPrevious JSON failed validation:\n${lastErrors}\nReturn only corrected JSON.`;

const result = await input.run(prompt);

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] lastSession is collected but never passed into the next provider call, so every schema retry starts a fresh agent context. Extend the provider input with providerSessionId and feed each attempt's returned ID into the next attempt. Also emit schema_retry only when another attempt will actually run.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed by PR #99 (a8b8e5a). providerSessionId is now part of the workflow provider input, each schema attempt carries forward the latest returned session ID, and the worker passes it into the provider adapter. schema_retry is emitted only when another attempt will actually run. Regression coverage verifies that attempt 2 receives the session ID from attempt 1.

source: "export const meta = { name: 'x', description: 'd' }\nreturn 1\n",
preferredName: "demo",
});
assert.match(path, /workflow-scripts\/wfr_test\/demo\.js$/);

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] This assertion is path-separator-specific and is the current Windows CI failure. Please compare normalized path segments, or compare against join('workflow-scripts', 'wfr_test', 'demo.js') after normalizing separators.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed by PR #99 (3343404). The persisted-script assertion now normalizes Windows separators before comparing the path suffix. The Windows smoke job is green on #99, #100, and #101, so this thread is obsolete after the stack lands.

Comment thread src/workflow-replay.ts
function toHit(call: WorkflowAgentCallRecord): WorkflowReplayHit {
if (call.structuredJson) {
try {
return {

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P0] Replay restores only the previous return value; it does not recreate filesystem side effects or reconnect the preserved worktree. A cached mutating agent can report success while the new run contains none of its changes. Please make mutating calls non-cacheable by default, or persist/apply a patch or resume the exact worktree as part of the replay contract.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant